refactor: deduplicate interpolator precompute and code4 matrix construction#2713
refactor: deduplicate interpolator precompute and code4 matrix construction#2713henryiii wants to merge 1 commit into
Conversation
…uction Factor the shared alphaset-dependent precompute block in code1 and code4 into a private _precompute_alphaset_dependent helper called by both _precompute and _precompute_alphasets, and drop the redundant bare returns. Factor the rank-6 A_inverse matrix literal (previously duplicated in code4.__init__ and rebuilt on every scalar call in _slow_code4.product) into a module-level _a_inverse(alpha0) helper, computed once per instance in _slow_code4. No change to math, shapes, or behavior. Assisted-by: ClaudeCode:claude-opus-4.8
|
Warning Review limit reached
More reviews will be available in 29 minutes and 55 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1 participant
🤖 AI text below 🤖
Part of the code review in #2706.
Summary
Deduplicates repeated code in
src/pyhf/interpolators/:code1.pyandcode4.py: thebases_up/bases_dn/mask_on/mask_off(plusonesin code4) block was duplicated verbatim between_precomputeand_precompute_alphasets. Extracted into a private_precompute_alphaset_dependent()helper called by both, keeping_precompute's extra histogram-only setup. Removed the redundant barereturnstatements at the ends of_precompute_alphasets.code4.py: the six-rowA_inversematrix literal appeared twice — once incode4.__init__and once inside_slow_code4.product, where it was rebuilt on every scalar call. Factored it into a module-level_a_inverse(alpha0)function returning the nested list, used by both. In_slow_code4it is now computed once per instance (alpha0is fixed at construction) instead of per call.Same math, same shapes, no behavior change.
Test plan
All run against the dev environment (strict pytest,
filterwarnings=error), all backends:pytest tests/test_interpolate.py -q→ 69 passed (includes slow-path vs fast-path agreement tests)pytest tests/test_validation.py tests/test_regression.py -q→ 26 passedprek -a(lint/format) → clean🤖 Generated with Claude Code